Skip to content

Shared rules implementation - #757

Draft
lbarcziova wants to merge 2 commits into
packit:mainfrom
lbarcziova:shared-rules-implementation
Draft

Shared rules implementation#757
lbarcziova wants to merge 2 commits into
packit:mainfrom
lbarcziova:shared-rules-implementation

Conversation

@lbarcziova

Copy link
Copy Markdown
Member

No description provided.

Implements a new MCP tool to fetch shared rules from a central registry,
allowing common guidelines to be defined once per ecosystem (Python, Perl,
etc.) and automatically applied to groups of packages.

Core changes:
- New SharedRulesTool: Looks up which shared rule sets apply to a package
  by querying registry.yaml in rules/shared-rules repo
- Returns JSON list of ecosystem names; agents fetch actual AGENTS.md
  content via existing get_maintainer_rules tool
- TTL-based caching (1 hour) at tool instance level for registry.yaml
- Centralize GITLAB_API_URL and RULES_NAMESPACE constants (currently
  pointing at ymir-rules-test for testing)
- Register tool in MCP gateway alongside existing MaintainerRulesTool
- Add PyYAML>=5.1 dependency for registry parsing
- Full test coverage (8 test cases) for lookup and caching behavior

Design rationale:
- Tool returns only the list, not content (keeps it simple)
- Agent orchestrates fetching (maintains control over precedence logic)
- Per-package rules always take precedence over shared rules
- Registry cached to avoid repeated GitLab API calls
- Transient errors don't cache; permanent errors do (404, bad YAML)

Assisted-by: Claude Opus 4.6
Update build fix prompt to clarify that both shared and package-specific
rules are checked when determining if adding BuildRequires/Requires is
allowed for backport build fixes.

Changes:
- Step 0: Clarify that rules (both shared and package-specific) are
  checked for BuildRequires permission, with package-specific taking
  precedence
- Criterion 2: Reference "rules from step 0" instead of just
  "maintainer rules"
- CRITICAL CONSTRAINTS: Use "rules" instead of "maintainer rules"
  throughout for consistency

This ensures that ecosystem-level permissions (e.g., "Python packages
can add BuildRequires for backport fixes") are respected, while still
allowing per-package overrides.

Assisted-by: Claude Opus 4.6

@TomasTomecek TomasTomecek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nicely done

async def _fetch_registry(self) -> dict[str, list[str]]:
if (
self._registry_fetched_at
and (time.monotonic() - self._registry_fetched_at) < REGISTRY_TTL_SECONDS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this definitely makes a lot of sense, we don't need to fetch the same file multiple times per minute

I do wonder though if we could just check if the file changed using the respective HTTP header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants